establish_channel_with_system#3721
Conversation
joepetrowski
left a comment
There was a problem hiding this comment.
I think the approach is OK, just a few naming comments.
| /// The default channel size and capacity to use when opening a channel to a system | ||
| /// parachain. | ||
| type DefaultChannelSizeAndCapacityToSystem: Get<(u32, u32)>; |
There was a problem hiding this comment.
I would add an integrity_test that these are below the maximum allowed by Configuration.
There was a problem hiding this comment.
config is from storages so it is not really possible to assert it at build time.
if the value is above the max defined in config, the call will fail, and I think that's fine.
There was a problem hiding this comment.
We do run integrity_test in externalities, so at least the genesis values can be checked.
There was a problem hiding this comment.
but why check with genesis values?
There was a problem hiding this comment.
Because they should also be correct? but its not a must, just an idea.
acatangiu
left a comment
There was a problem hiding this comment.
approach looks good, will approve once PR is final (todos, tests, benchmarks, etc)
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
|
The CI pipeline was cancelled due to failure one of the required jobs. |
| // Measured: `417` | ||
| // Estimated: `6357` | ||
| // Minimum execution time: 629_674_000 picoseconds. | ||
| Weight::from_parts(640_174_000, 0) |
There was a problem hiding this comment.
copied from establish_system_channel
|
not sure why PRDoc check failed? |
You need to specify what kind of bump should the affected crates get: polkadot-sdk/prdoc/pr_3927.prdoc Line 13 in 665e365 |
Co-authored-by: Adrian Catangiu <adrian@parity.io>
Implements polkadot-fellows/RFCs#82 Allow any parachain to have bidirectional channel with any system parachains Looking for initial feedbacks before continue finish it TODOs: - [x] docs - [x] benchmarks - [x] tests --------- Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com> Co-authored-by: Adrian Catangiu <adrian@parity.io>
Allows any parachain to have bidirectional channel with any system parachains. Relates to: polkadot-fellows/RFCs#82 Relates to: paritytech/polkadot-sdk#3721 Relates to: paritytech/polkadot-sdk#4332 <!-- Remember that you can run `/merge` to enable auto-merge in the PR --> <!-- Remember to modify the changelog. If you don't need to modify it, you can check the following box. Instead, if you have already modified it, simply delete the following line. --> - [x] regenerate weights for `runtime_parachains::hrmp` - [ ] Does not require a CHANGELOG entry ## Polkadot weights (the threshold moved from the default 5 to 0.1 to see some differences) ``` subweight compare commits --path-pattern "./relay/polkadot/**/weights/**/*.rs" --format markdown --no-color --change added changed --method asymptotic --ignore-errors --threshold 0.1 remotes/polkadot-fellows/main origin/bko-hrmp-patch ``` | File | Extrinsic | Old | New | Change [%] | |-------------------------------------------------------|-------------------------------|----------|----------|------------| | relay/polkadot/src/weights/runtime_parachains_hrmp.rs | poke_channel_deposits | 137.20us | 140.03us | +2.06 | | relay/polkadot/src/weights/runtime_parachains_hrmp.rs | hrmp_accept_open_channel | 555.89us | 563.03us | +1.28 | | relay/polkadot/src/weights/runtime_parachains_hrmp.rs | hrmp_close_channel | 556.97us | 563.79us | +1.22 | | relay/polkadot/src/weights/runtime_parachains_hrmp.rs | hrmp_init_open_channel | 732.03us | 740.10us | +1.10 | | relay/polkadot/src/weights/runtime_parachains_hrmp.rs | force_open_hrmp_channel | 1.16ms | 1.17ms | +0.98 | | relay/polkadot/src/weights/runtime_parachains_hrmp.rs | establish_system_channel | 1.15ms | 1.16ms | +0.69 | | relay/polkadot/src/weights/runtime_parachains_hrmp.rs | force_process_hrmp_open | 102.05ms | 102.47ms | +0.42 | | relay/polkadot/src/weights/runtime_parachains_hrmp.rs | force_clean_hrmp | 91.52ms | 91.86ms | +0.37 | | relay/polkadot/src/weights/runtime_parachains_hrmp.rs | clean_open_channel_requests | 16.56ms | 16.61ms | +0.35 | | relay/polkadot/src/weights/runtime_parachains_hrmp.rs | force_process_hrmp_close | 75.41ms | 75.65ms | +0.33 | | relay/polkadot/src/weights/runtime_parachains_hrmp.rs | hrmp_cancel_open_request | 422.12us | 415.03us | -1.68 | | relay/polkadot/src/weights/runtime_parachains_hrmp.rs | establish_channel_with_system | | 1.67ms | Added | ## Kusama weights (the threshold moved from the default 5 to 0.1 to see some differences) ``` subweight compare commits --path-pattern "./relay/kusama/**/weights/**/*.rs" --format markdown --no-color --change added changed --method asymptotic --ignore-errors --threshold 0.1 remotes/polkadot-fellows/main origin/bko-hrmp-patch ``` | File | Extrinsic | Old | New | Change [%] | |-----------------------------------------------------|-------------------------------|----------|----------|------------| | relay/kusama/src/weights/runtime_parachains_hrmp.rs | poke_channel_deposits | 137.23us | 140.25us | +2.20 | | relay/kusama/src/weights/runtime_parachains_hrmp.rs | hrmp_close_channel | 557.39us | 564.18us | +1.22 | | relay/kusama/src/weights/runtime_parachains_hrmp.rs | hrmp_accept_open_channel | 556.49us | 563.23us | +1.21 | | relay/kusama/src/weights/runtime_parachains_hrmp.rs | hrmp_init_open_channel | 735.26us | 743.59us | +1.13 | | relay/kusama/src/weights/runtime_parachains_hrmp.rs | force_open_hrmp_channel | 1.16ms | 1.17ms | +0.97 | | relay/kusama/src/weights/runtime_parachains_hrmp.rs | establish_system_channel | 1.15ms | 1.16ms | +0.84 | | relay/kusama/src/weights/runtime_parachains_hrmp.rs | hrmp_cancel_open_request | 413.79us | 416.03us | +0.54 | | relay/kusama/src/weights/runtime_parachains_hrmp.rs | force_process_hrmp_open | 101.96ms | 102.43ms | +0.46 | | relay/kusama/src/weights/runtime_parachains_hrmp.rs | clean_open_channel_requests | 16.54ms | 16.61ms | +0.45 | | relay/kusama/src/weights/runtime_parachains_hrmp.rs | force_clean_hrmp | 91.43ms | 91.82ms | +0.43 | | relay/kusama/src/weights/runtime_parachains_hrmp.rs | force_process_hrmp_close | 75.34ms | 75.63ms | +0.40 | | relay/kusama/src/weights/runtime_parachains_hrmp.rs | establish_channel_with_system | | 1.67ms | Added |
Implements polkadot-fellows/RFCs#82
Allow any parachain to have bidirectional channel with any system parachains
Looking for initial feedbacks before continue finish it
TODOs: